Skip to content

Make Sync Gateway version comparison robust for dev builds#398

Merged
torcolvin merged 5 commits into
mainfrom
improve-version-comparison
May 20, 2026
Merged

Make Sync Gateway version comparison robust for dev builds#398
torcolvin merged 5 commits into
mainfrom
improve-version-comparison

Conversation

@torcolvin
Copy link
Copy Markdown
Collaborator

@torcolvin torcolvin commented Apr 23, 2026

Goal: Allow code that uses supports_version_vectors() to pass with Sync Gateway dev builds, right now it throws an exception because get_version returns "unknown" for dev builds

Sync Gateway has some unfortunate behavior around reporting version numbers for dev builds. This is probably a bug in Sync Gateway but a dev build and a release build return different values.

For a dev build:

    "vendor": {
        "name": "Couchbase Sync Gateway",
        "version": "4.1"
    },
    "version": "Couchbase Sync Gateway/mybranch(9fb8653) EE"

For a release build:

    "vendor": {
        "name": "Couchbase Sync Gateway",
        "version": "4.0"
    },
    "version": "Couchbase Sync Gateway/4.0.4(8;release) EE"

Therefore, try to use version so that the version number has build number but fall back to vendor.version for dev builds. We probably never want to upload this to greenboard and if we did we'd want something to actually say the commit hash but that doesn't work with the build number being integer typed. I'm not trying to fix that problem in this ticket though.

@borrrden, looking for your input on whether you are OK using pydantic for json parsing.
@vipbhardwaj, I ran most of the QE tests but you should make sure this code works before I break Jenkins by mistake.

@torcolvin torcolvin requested a review from Copilot April 23, 2026 16:20
Copilot AI reviewed Apr 23, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR aims to make Sync Gateway feature gating (specifically supports_version_vectors()) work with Sync Gateway dev builds where the reported version string differs from release builds, by switching to / _status parsing and centralizing the version-vector capability check.

Changes:

  • Add SyncGatewayStatusResponse/vendor parsing (via pydantic) and update get_version() to use / _status.
  • Introduce supports_version_vectors() and update QE tests to call it instead of parsing SGW versions inline.
  • Make Sync Gateway public port configurable and thread it through replication/public API calls.

Reviewed changes

Copilot reviewed 6 out of 7 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
client/src/cbltest/api/syncgateway.py Adds / _status parsing + supports_version_vectors(), and introduces public_port plumbing.
tests/QE/test_xattrs.py Switches VV gating to supports_version_vectors().
tests/QE/test_users_channels.py Switches VV gating to supports_version_vectors().
tests/QE/test_replication_multiple_clients.py Switches VV gating to supports_version_vectors() for one test section.
tests/QE/test_multiple_servers.py Switches VV gating to supports_version_vectors().
client/pyproject.toml Adds pydantic dependency.
uv.lock Locks pydantic (and transitive deps) and updates typing-extensions.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread client/src/cbltest/api/syncgateway.py
Comment thread client/src/cbltest/api/syncgateway.py Outdated
Comment thread client/src/cbltest/api/syncgateway.py Outdated
Comment thread client/src/cbltest/api/syncgateway.py Outdated
@torcolvin torcolvin force-pushed the improve-version-comparison branch from 34ed18d to 377ef27 Compare May 5, 2026 12:38
@torcolvin torcolvin requested review from borrrden and vipbhardwaj May 5, 2026 13:29
vipbhardwaj
vipbhardwaj approved these changes May 6, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@vipbhardwaj vipbhardwaj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

borrrden
borrrden requested changes May 7, 2026
View reviewed changes
Comment thread client/src/cbltest/api/syncgateway.py Outdated
Comment thread client/src/cbltest/api/syncgateway.py
Comment thread client/src/cbltest/api/syncgateway.py Outdated
Comment thread client/src/cbltest/api/syncgateway.py Outdated
@torcolvin
Copy link
Copy Markdown
Collaborator Author

torcolvin commented May 8, 2026

I made an accidental mistake by testing not quite prod versions of Sync Gateway from latestbuilds, so I would still love if someone can double check that a test like test_offline_processing_of_external_updates is not broken.

@torcolvin torcolvin requested a review from borrrden May 8, 2026 01:59
@borrrden
Copy link
Copy Markdown
Member

borrrden commented May 11, 2026

That tests still works but I had to disable TLS verification. It seems like it has recently gotten stricter. The CA cert is going to have to be regenerated at some point to include proper extensions.

@torcolvin
Copy link
Copy Markdown
Collaborator Author

torcolvin commented May 12, 2026

That tests still works but I had to disable TLS verification. It seems like it has recently gotten stricter. The CA cert is going to have to be regenerated at some point to include proper extensions.

When you say disable TLS verification, where did you do that? I don't make any new calls, I just use resp = await self._send_request("get", "/_status")

Do you mean disabling TLS here

couchbase-lite-tests/client/src/cbltest/api/syncgateway.py

Line 594 in 62153cb

def _create_session(
? If you wanted to do that this seems like a separate PR.

@borrrden
Copy link
Copy Markdown
Member

borrrden commented May 12, 2026

That tests still works but I had to disable TLS verification. It seems like it has recently gotten stricter. The CA cert is going to have to be regenerated at some point to include proper extensions.

When you say disable TLS verification, where did you do that? I don't make any new calls, I just use resp = await self._send_request("get", "/_status")

Do you mean disabling TLS here

couchbase-lite-tests/client/src/cbltest/api/syncgateway.py

Line 594 in 62153cb

def _create_session(

? If you wanted to do that this seems like a separate PR.

I meant the latter. The CA cert in the repo doesn't have extensions properly set and so is getting rejected by python 3.13. It definitely is a separate PR to handle that (it means recreating the CA cert and leaf certs)

@torcolvin torcolvin merged commit 6c111db into main May 20, 2026
5 checks passed
@torcolvin torcolvin deleted the improve-version-comparison branch May 20, 2026 15:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@borrrden borrrden borrrden approved these changes

@vipbhardwaj vipbhardwaj vipbhardwaj approved these changes

Assignees

@borrrden borrrden

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@torcolvin @borrrden @vipbhardwaj